Skip to content

Conversation

@sanderegg
Copy link
Member

@sanderegg sanderegg commented Jun 27, 2025

What do these changes do?

This pull request refactors the autoscaling service to improve modularity and maintainability. Its main purpose is to simplify and de-confuse developers with regard to old naming conventions (namely usage of hot/warm buffer namings as per the current conventions).

There are no functionalities changed but the metrics names that the autoscaling-overview dashboard uses: related PR for devops are below.

Related issue/s

How to test

Dev-ops

@sanderegg sanderegg added this to the Engage milestone Jun 27, 2025
@sanderegg sanderegg self-assigned this Jun 27, 2025
@sanderegg sanderegg added t:maintenance Some planned maintenance work a:autoscaling autoscaling service in simcore's stack labels Jun 27, 2025
@codecov
Copy link

codecov bot commented Jun 27, 2025

Codecov Report

Attention: Patch coverage is 93.88646% with 14 lines in your changes missing coverage. Please review.

Project coverage is 88.11%. Comparing base (961ad38) to head (ef60cad).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8002      +/-   ##
==========================================
- Coverage   88.23%   88.11%   -0.13%     
==========================================
  Files        1863     1856       -7     
  Lines       71964    71795     -169     
  Branches     1261     1261              
==========================================
- Hits        63496    63260     -236     
- Misses       8104     8171      +67     
  Partials      364      364              
Flag Coverage Δ
integrationtests 52.55% <ø> (-11.66%) ⬇️
unittests 86.88% <93.88%> (+0.02%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library 93.93% <ø> (ø)
pkg_celery_library 87.15% <ø> (ø)
pkg_dask_task_models_library 79.62% <ø> (ø)
pkg_models_library 93.24% <ø> (ø)
pkg_notifications_library 85.26% <ø> (ø)
pkg_postgres_database 88.22% <ø> (ø)
pkg_service_integration 69.92% <ø> (ø)
pkg_service_library 71.25% <ø> (ø)
pkg_settings_library 90.64% <ø> (ø)
pkg_simcore_sdk 85.05% <ø> (ø)
agent 96.29% <ø> (ø)
api_server 92.83% <ø> (ø)
autoscaling 95.88% <93.88%> (-0.15%) ⬇️
catalog 92.58% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 92.35% <ø> (ø)
datcore_adapter 97.94% <ø> (ø)
director 76.73% <ø> (ø)
director_v2 91.04% <ø> (ø)
dynamic_scheduler 96.69% <ø> (ø)
dynamic_sidecar 90.09% <ø> (ø)
efs_guardian 89.65% <ø> (ø)
invitations 93.60% <ø> (ø)
payments 92.57% <ø> (ø)
resource_usage_tracker 92.47% <ø> (+0.26%) ⬆️
storage 86.72% <ø> (+0.04%) ⬆️
webclient ∅ <ø> (∅)
webserver 87.99% <ø> (-0.55%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 961ad38...ef60cad. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sanderegg sanderegg force-pushed the autoscaling/refactoring branch 3 times, most recently from 2cab411 to 3de8413 Compare July 4, 2025 06:26
@sanderegg sanderegg changed the title ♻️Autoscaling: refactor before changes ♻️Autoscaling: refactor before changes (⚠️ DEVOPS) Jul 4, 2025
@sanderegg sanderegg requested a review from YuryHrytsuk July 4, 2025 12:39
@sanderegg sanderegg marked this pull request as ready for review July 4, 2025 12:41
@sanderegg sanderegg requested a review from pcrespov as a code owner July 4, 2025 12:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the autoscaling service by replacing legacy “buffer” naming with distinct “warm”/“hot” buffers, and by modularizing autoscaling modes into provider classes.

  • Introduces ComputationalAutoscalingProvider and DynamicAutoscalingProvider in place of BaseAutoscaling subclasses
  • Renames buffer-related utilities and model fields (bufferwarm_buffer/hot_buffer) and updates EC2 tag helpers to operate on EC2Tags only
  • Updates imports and paths for cluster‐scaling, instrumentation, and tasks modules

Reviewed Changes

Copilot reviewed 31 out of 32 changed files in this pull request and generated no comments.

Show a summary per file
File Description
utils/warm_buffer_machines.py Extracted buffer‐tag helpers; renamed and simplified tag functions
utils/utils_ec2.py Added DNS parsing helpers; standardized logging and regex for private DNS
utils/utils_docker.py Added helper to detect drained instances; simplified tag maps
utils/cluster_scaling.py Renamed buffer functions to hot/warm; removed old DNS helpers
modules/cluster_scaling/_provider_dynamic.py Converted static methods to instance providers
modules/cluster_scaling/_provider_computational.py Converted static methods to instance providers
modules/cluster_scaling/_buffer_machines_pool_core.py Switched buffer pool types to warm; updated tag usages
modules/cluster_scaling/_auto_scaling_core.py Switched buffer pools to warm/hot; updated provider protocol usage
Numerous test files under tests/unit Updated imports and renamed test fixtures to match hot/warm naming
core/application.py Pointed background tasks setup to new cluster_scaling paths
Comments suppressed due to low confidence (2)

services/autoscaling/src/simcore_service_autoscaling/modules/cluster_scaling/_buffer_machines_pool_core.py:309

  • Wrap the tag value in AWSTagValue for consistency and validation (e.g. AWSTagValue("true")) rather than using a raw string literal.
                BUFFER_MACHINE_PULLING_EC2_TAG_KEY: "true",

services/autoscaling/src/simcore_service_autoscaling/utils/utils_docker.py:280

  • [nitpick] Consider extracting the reservation and limit values into helper variables or a small function to reduce nested parentheses and improve readability, e.g.: res = task.spec.resources or Resources(); cpu_res = res.reservations.nano_cp_us or 0; cpu_lim = res.limits.nano_cp_us or 0; cpus = max(cpu_res, cpu_lim) / _NANO_CPU.
            cpus=max(

@sanderegg sanderegg added the 🤖-automerge marks PR as ready to be merged for Mergify label Jul 4, 2025
@sanderegg sanderegg force-pushed the autoscaling/refactoring branch from 1ef39e3 to 4dad4e7 Compare July 4, 2025 15:59
@sanderegg
Copy link
Member Author

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Jul 4, 2025

queue

🛑 The pull request has been removed from the queue default

The following conditions don't match anymore:

  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • any of: [🛡 GitHub branch protection]
        • check-neutral = unit-tests
        • check-skipped = unit-tests
        • check-success = unit-tests

@sanderegg
Copy link
Member Author

@mergify requeue

@mergify
Copy link
Contributor

mergify bot commented Jul 7, 2025

requeue

☑️ This pull request is already queued

@sanderegg sanderegg force-pushed the autoscaling/refactoring branch from 4dad4e7 to 567266b Compare July 7, 2025 05:45
@mergify
Copy link
Contributor

mergify bot commented Jul 7, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@sanderegg sanderegg force-pushed the autoscaling/refactoring branch from 30b7fe2 to 6083c55 Compare July 7, 2025 08:12
@sanderegg
Copy link
Member Author

@mergify requeue

@mergify
Copy link
Contributor

mergify bot commented Jul 7, 2025

requeue

✅ This pull request will be embarked automatically

The head sha of this pull request, 6083c55, was never embarked in the merge queue.

But don't worry, Mergify will embark it automatically for you.

@sanderegg sanderegg force-pushed the autoscaling/refactoring branch from 482daf4 to d8a0d4b Compare July 7, 2025 09:55
Copy link
Contributor

@YuryHrytsuk YuryHrytsuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Copilot's summary shed some light on exact changes. It was helpful this time 🚀

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 7, 2025

@mergify
Copy link
Contributor

mergify bot commented Jul 7, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@sanderegg sanderegg merged commit bbef4bc into ITISFoundation:master Jul 7, 2025
94 of 98 checks passed
@sanderegg sanderegg deleted the autoscaling/refactoring branch July 7, 2025 12:29
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:autoscaling autoscaling service in simcore's stack t:maintenance Some planned maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants